-
Notifications
You must be signed in to change notification settings - Fork 98
Fix #20 - add support for <welcome-servlet> #881
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Re-based and made a couple clarifications to the specification text. No functional change. |
Some feedback from the Tomcat community: At the moment, a Is this a problem? I think probably yes, but would welcome additional views. If it is an issue, then one way to resolve it would be to change the mapping algorithm to: For each welcome resource in the order it appears in the deployment descriptor:
If no match is found then for each legacy welcome file and
This does mean two passes of the welcome files are more likely but I think it is closer to the current behaviour whilst fixing the problem this change was intended to fix. I'm currently leaning towards this alternative approach. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, coming at this very late.
I'm not understanding why in the 6.2 descriptor that welcome-file
resources can be matched to servlets?
Also, I don't think this well handles the case of '*.jsp', where if a file exists AND there is a servlet mapping, then dispatch to the servlet.
- if the welcome URI has an exact match to a servlet mapping, send the request | ||
to that servlet | ||
- if the welcome URI has a prefix match to a servlet mapping, send the request | ||
to that servlet |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we mapping welcome-file elements in 6.2 descriptors to servlets? Surely these mappings should only be for welcome-servlet
resources and perhaps legacy welcome files?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The second bullet (exact matching) is to handle pre-compiled JSPs.
The third bullet (prefix mapping) is probably unnecessary. There might be some backwards compatibility edge cases but I don't have a concrete argument for keeping it.
I see the point re *.jsp. I think that is a wording issue. How about this instead of the last bullet for the first iteration:
- if a static file exists in the web application for the welcome URI, map the URI to a servlet using the standard mapping rules (including the default servlet)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would think that for all welcome-file
resources, then the file must exist. This would work for precompiled JSPs because the index.jsp file should still be there. Then only for welcome-servlet
resources do we look for a match regardless of the files existence?
I'm AFK until Monday, so can this wait until then for some more considered discussion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On #20 @janbartel expressed the view that after pre-compilation, the original JSP may not exist. Hence the current wording. I don't know how prevalent that scenario is. If we were able to limit welcome-file
to files that exist that would simplify both the specification text and the implementation.
I'm in no particular rush to merge this. I'd rather take the time to get it right.
Implements the solution discussed in #20.
I plan on leaving this open for a week or two for discussion before merging and wouldn't be surprised if changes were required.